Nightly audit: 1 issue(s) - #47
Conversation
…rammar post-process Implements the design audit's prescription end-to-end (findings #1–#15 + N1/N2/N7/N9/N10 + follow-ups). The product's chain-of-custody promise — every Phase 2 recommendation traces back to the Phase 1 measurement that justifies it — is now visually first-class on every card, not a 9px monospace footnote. What ships: * Chain-of-custody (findings #2 + #3). New CitationBlock primitive renders a structured "GROUNDED IN" block above every Mix Chain / Patches / Sonic Element card with humanized labels (FIELD_LABELS map, ~50 entries + humanizeFieldPath fallback) and a ConfidenceBandBadge pill computed from the worst confidence among cited fields. Also retires GroundingBadgeList at the Track Layout site; segmentIndexes ride through a new extraRows prop. * Recommendations-first IA (#1). MeasurementDashboard moved to the bottom of the results scroll; StickyNav's 9 measurement pills collapse to one trailing "Measurements" entry. Producers hit Style → Sonic → Mix Chain → Patches → Session before the measurement evidence. * Header polish (#7, #9, #11). CPU meter removed (browser-tab CPU is misleading during backend analysis), "Local DSP Engine v1.6.0" eyebrow removed (resolves mobile 3-line wrap), Dense DAW Lab demoted from accent chip to quiet text link. * Engineering vocab cleanup (#8 + N3/N4/N5/N8). New userLabels.ts service translates field paths to producer-readable labels at every render. Button labels renamed (Download data / Download report). FAMILY: NATIVE chip dropped from meta-badge rows. workflowStage prettified at the view-model layer ("Sound design" not "SOUND_DESIGN"). AI Interpretation gated copy reworded ("AI interpretation isn't configured…" not "Developer kill-switch is off"). * Applied-recommendations tracker (#14 + #15). Per-card checkbox affordance + section- header "N of M applied" chip + localStorage persistence keyed by audio content SHA256. Producer can rename their file without losing their progress. * Idle value-prop panel (#5). Replaces the 200px "NO SIGNAL DETECTED" canvas with a producer-readable explanation of what ASA does, with honest pacing copy (4–5 min Phase 2 wait, not 2–5 min). * Patches group structure. Mirrors Mix Chain's emoji-eyebrow grouping (Drums / Bass / Synth / Master) so producers can jump to the bass patch without scanning 8 cards. * Input Source collapse (N9). Post-analysis, the Input Source panel collapses to a compact summary with filename + duration + "Analyze new file" + "Adjust settings". Frees the top of the page for the results the user came for. * AnalysisStatusPanel primary readout (#6). Stage diagnostic message promoted from 9px footnote to the visual focus of the progress card. Pre-existing tone-aware fill (running / success / failed) preserved. * Phase 2 failure mode (N1). Header subtitle derives from interpretation stage status (no more "PHASE COMPLETE" while INTERPRET still RUNNING/FAILED). StickyNav Phase 2 pills render disabled with hover-reason when sections didn't populate. Retry button gated on error.retryable; non-retryable failures surface the error code inline. * Misc audit follow-ups: BPM reconciled across exec card + Core Metrics tile (N2); Signal Monitor STANDBY canvas hidden when audio isn't playing, freeing ~160px (N7); StickyNav label "Device Chain" → "Sections" (N10, less ambiguous with Ableton's own effects-routing meaning); BASS group icon swapped from 🫧 → Lucide AudioWaveform (#13); toggle helper paragraphs switched from all-caps mono walls to sans-serif sentence case (#4 revised). * Phase 2 grammar post-process (audit final round). The prompt instruction added earlier didn't take — Gemini still emits "by recreates / by absorbs / by shapes" 3rd-person singular forms after "by" in role/reason text. Server-side _apply_phase2_grammar_fixes rewrites these to gerunds in-place on mixAndMasterChain[].reason, abletonRecommendations[].{reason,advancedTip}, and secretSauce.workflowSteps[].{instruction,measurementJustification}. Conservative regex (\bby \w{4,}s\b) + denylist guards against plural-noun false positives. Test coverage: * UI: 46 test files / 540 tests pass (was 39/422 before the audit). New service tests: userLabels (21), phase1Picker (25), citationBlock (12), appliedRecommendations (16), formatTrackDuration (12), interpretationSubtitle (10), workflowStagePrettifier (8), analysisStatusProgress (6), idleValuePropPanel + phase2NavReason. New DOM tests in analysisResultsUi.test.ts cover Track Layout citation, applied-checkbox flow, mix- chain citation rendering. * Backend: 16 new unit tests in test_phase2_grammar_fix.py cover _to_gerund, _fix_by_gerund_in_text, _apply_phase2_grammar_fixes including the actual screenshot corpus (recreates → recreating, shapes → shaping, matches → matching, etc.). Full suite: 463 of 463 ASA tests pass; 1 unrelated pre-existing failure in tests.test_url_ingest predates this branch. Visual verification: Playwright capture pass against a real 126s track confirmed every surface (15 screenshots in /tmp/asa-shots-audit-final/). Phase 2 returned in 220s; localStorage round-trip verified on applied-checkbox toggles. Documented limitations: * The gerund rule is algorithmic — verbs requiring consonant doubling (control → controlling, submit → submitting) degrade to "controling" / "submiting". Still better than "by controls". Drop a hand-mapped exception into the module if observed in real output. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI surfaced 7 smoke test failures from this branch's audit changes that hadn't been propagated to the smoke spec assertions: * `tests/smoke/ui-details.spec.ts` × 3 - `NO SIGNAL DETECTED` → `IdleValuePropPanel` (audit #5) - `JSON_DATA` / `REPORT_MD` button labels → `Download data` / `Download report` * `tests/smoke/responsive-layout.spec.ts` × 4 - `NO SIGNAL DETECTED` (×2) → `IdleValuePropPanel` - `CPU` text-presence checks removed; the two viewport-shape tests now assert just the model-selector responsive behavior (audit #11 retired the CPU meter; there's no element to assert) * `tests/smoke/file-validation.spec.ts` × 1 - `re-upload after results resets to file-selected state`: the test used `Remove File` (FileUpload component's affordance) to clear after results were visible. Post-N9 collapse, the Input Source panel replaces FileUpload with a compact summary card whose "↺ Analyze new file" button calls the same handleFileClear. Switched the test to target the new affordance. Also updated the e2e exports spec for label consistency (not in the failing CI job, but the same renames apply): * `tests/e2e/phase1-exports.spec.ts` - `downloadTextArtifact(page, /JSON_DATA/i)` → `/Download data/i` - `downloadTextArtifact(page, /REPORT_MD/i)` → `/Download report/i` Verified locally against the live stack: 45 of 46 smoke tests pass, 1 skipped (was unrelated). The previously-failing 7 are all green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 unit tests failing — all fixture-dependent live decision-gate comparators that assert instead of skipping when Gemini snapshots are absent. No Phase-boundary violations found. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
slittycode
left a comment
There was a problem hiding this comment.
Verdict: COMMENT
Summary
Large mixed PR: a backend grammar post-processor for Phase 2 text, substantial frontend UX improvements (CitationBlock, applied-recommendations tracker, progress-card honesty fixes, nav reorder, idle panel), and the nightly audit report documenting 3 pre-existing test failures from PR #18. All new logic has test coverage. Phase boundary is clean. Phase 1 schema is untouched. The 3 failing tests predate this PR and are flagged correctly in the description — they're the only thing standing between npm test being green.
Findings
Should fix (not introduced by this PR, but blocks CI)
apps/ui/tests/decision_gate.*.live.test.ts (three files from PR #18) — the docstrings say "Skips when no snapshots are present" but the code unconditionally asserts expect(ok.length).toBeGreaterThan(0), which fails on any machine without pre-generated Gemini snapshots. npm test is red on CI until this is resolved. The fix is one of: (a) gate behind RUN_GEMINI_LIVE_SMOKE=true (matching the existing smoke pattern), or (b) add a guard that calls it.skip when the fixture directory is absent. Either is fine; the current state is not.
Worth considering
server_phase2.py — _to_gerund doesn't handle consonant doubling (controls → "controling" instead of "controlling"). Documented in the test comments. If Gemini starts emitting "by controls" the output gets worse, not better. A small exception dict at module level for the 5–10 most common affected verbs would close the gap without trying to implement English orthographic rules.
server_phase2.py:111 — return record if updated or True else record — the updated flag is computed but the or True makes it a no-op. Dead code; consider just return record.
appliedRecommendations.ts — no max-entries or TTL guard on the localStorage store. Orphaned records accumulate over many sessions. Low priority for V1 (acknowledged in the docstring), but if a producer runs ASA on 50 tracks, the key will hold 50 stale entries forever. A simple prune-to-N-most-recent on write would contain it.
Test results
Unit: 3 failed / 557 passed — all 3 failures are in decision_gate.*.live.test.ts (pre-existing, PR #18). npm run test:unit is green.
Playwright smoke: 44 passed / 2 skipped / 0 failed.
Phase boundary check
Clean.
_apply_phase2_grammar_fixesmutates the normalized Phase 2 dict in-place. Never touches Phase 1 data.CitationBlock/phase1Picker.tsread Phase 1 via dotted-path traversal, strictly read-only.appliedRecommendations.tsis a UI concern (localStorage); no contact with either measurement layer.CONFIDENCE_PAIRSexport fromphase2Validator.tsis a correct refactor — single source of truth now shared by the picker and the validator.Math.round(phase1.bpm)inMeasurementDashboard.tsxis display-only; the underlyingphase1.bpmfloat is unchanged.
Generated by Claude Code
Nightly audit — 2026-05-14
Automated Phase-boundary + test-guardrail audit.
Result
vitest run)playwright test)Repo-structure note (task assumptions vs. reality)
The task brief assumes
src/phase2/andsrc/phase3/directories and a root-levelnpm ci/ Essentia.js. The actual repo does not match:package.json. Frontend deps live inapps/ui; the setupscript's
npm ciwas run there instead.apps/backend(analyze*.py), notEssentia.js. It is the ground-truth measurement layer.
apps/backend/server_phase2.py+ Gemini, with thefrontend guardrail in
apps/ui/src/services/phase2Validator.ts.patchSmith.tsis still the open item inBACKLOG.md). Nothing to audit there.The audit was performed against the real structure.
Failing tests
All 3 failures are in fixture-dependent live decision-gate comparators, all
introduced together in commit
94a26fbd(PR #18):apps/ui/tests/decision_gate.multi.live.test.ts:147—
decision-gate (multi-model) compares validator output across Gemini variantsapps/ui/tests/decision_gate.real.live.test.ts:152—
decision-gate (multi-model, real track) ... on Vtss-CantCatchMe.mp3apps/ui/tests/decision_gate.stems.live.test.ts:168—
decision-gate (multi-model, stem-aware, real track) ... with stemAnalysis populatedFailure mode (all three):
AssertionError: expected 0 to be greater than 0at
expect(ok.length).toBeGreaterThan(0). Each test loads pre-generated Geminisnapshots (
/tmp/decision_gate_<model>.jsonorapps/backend/.runtime/reports)produced by live multi-model Gemini runs. With no snapshots present,
ok.lengthis 0 and the assertion fails.
Root cause: test-harness bug, not a product regression. Each file's own
docstring claims it "Skips when no snapshots are present, so the suite stays
clean elsewhere" — but the code asserts
toBeGreaterThan(0)instead of callingit.skip/this.skip(). The intended skip-when-no-fixtures behavior was neverimplemented.
Not a Phase-boundary violation. These tests exercise the boundary guardrail
(
phase2Validator.ts) read-only; they don't cross it.Note:
npm run test:unit(scoped totests/services) is green — these threelive tests sit at
tests/root and are only picked up by the unscopednpm test.Phase-boundary scan — no violations
Checked every path where Phase 2 / interpretation touches Phase 1 / measurement
data:
apps/backend/analysis_runtime.py) — measurement resultslive in the
measurement_outputstable; interpretation results live in theseparate
interpretation_attemptstable._update_measurement_rowis calledonly from the measurement stage (
analysis_runtime.py:615,686,704) andprogress updaters in
server.py. The interpretation stage references themeasurement via FK
grounded_measurement_output_id— a read-only pointer,never a write-back.
server_phase2.py:742_normalize_measurement_result_for_gemini— buildsdict(payload)/dict(stem_entry)copies and only renames spectral keys(
spectralCentroid→spectralCentroidMean, etc.) for prompt presentation soGemini's field names match the validator contract. Operates on copies; the
stored measurement is untouched. No recompute of values.
apps/ui/src/services/analysisRunsClient.ts:184projectPhase1FromRun—returns
{ ...measurement, transcriptionDetail: pitchNote }: a new object thatattaches the Layer-2 pitch/note result as an additive field. No measured DSP
value is overwritten; no mutation of the source object.
apps/ui/src/services/phase2Validator.ts— reads Phase 1 strictlyread-only to check Phase 2 consistency. This is the boundary guardrail.
Phase 1 → Phase 2 contract — OK
Phase1Result(apps/ui/src/types/measurement.ts) flows into Phase 2 only as:(a) read-only input to the Gemini prompt builder, and (b) read-only input to
validatePhase2Consistency. No downstream writer assigns to tonal-balance,dynamics-envelope, stereo-image, loudness (LUFS/LRA/peak), or spectral
centroid/rolloff fields.
Needs human review
decision_gate.*.live.test.tsskip logic. Decide intended behavior:either gate these behind an env flag (e.g.
RUN_GEMINI_LIVE_SMOKE, as thesmoke suite already does) or have them
it.skipwhen no snapshots/fixtures arepresent — matching their own docstrings. Until then
npm testis red on anymachine without pre-generated Gemini snapshots, including CI.
Suggested next steps (no auto-fix applied)
under an env gate so the default
npm teststays green.npm testandnpm run test:unitscope, or document that thenightly guardrail should run
test:unitfor the deterministic subset.Generated by the
asa-nightly-phase-1-invariant--test-guardrailscheduled task.No source files were modified; no fixes were applied.